-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added filter function to @autodocs #885
Conversation
Awesome, thanks! I don't think this really needs to be more complicated than this, so the implementation looks good. Just needs docs as you mentioned. I am not entirely sure what are the types of the objects that will get passed to the filter function though -- it would be good to document that. For functions you probably get these Does it also work if you create a lambda function directly in the at-autodocs block? For tests, I think we should add something to |
Great! Concerning lambda functions: No, unfortunately not. Providing a lambda function raises one of these world age errors, that I have never really understood. I assume there is somewhere a I will add a description to the docs and my example to the tests. But that will take me a moment as I don't have access to a real computer at the moment (: |
Probably just need to invoke it with |
@fredrikekre Thanks, I will try when updating the PR. Would be great if lambda functions worked! |
@mortenpi , I updated the PR with lambda functions now working, an example in tests and a note in the documentation. Could you check whether you find it okai? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome, thanks!
Would you mind adding a brief description of this feature to CHANGELOG.md, under a new "Version v0.21.0" section? Scratch that -- I added it in #896. But wording suggestions welcome.
@@ -85,6 +85,27 @@ docstrings. Note that page matching is done using the end of the provided string | |||
`a.jl` will be matched by *any* source file that ends in `a.jl`, i.e. `src/a.jl` or | |||
`src/foo/a.jl`. | |||
|
|||
To filter out certain docstrings by your own criteria, you can provide function with them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/them/the ?
Hmm, I started wondering about what gets passed as the argument to the lambda function. E.g. if you add the following into the
you get this error
Could we have a short docs section on what are the types that will be used to call the filtering function and how to handle them? Also, I was hoping that |
As far as I understand, And the description in the CHANGELOG.md sounds very good to me, thanks!
|
@mortenpi I modified the code so the That should conclude the PR? On a side note: I would suggest to refactor the evaluation of the keywords a bit in order to avoid the for loops and if clauses which made it a bit hard for me to find the error. But that would be separate PR, I think. Still, would you generally be open for that, @mortenpi ? Then I would make a suggestion. |
Yes, this is good to go. Thanks for iterating on this!
That sounds good to me. If you'd be willing to make a PR, that would be awesome! |
Hi, I have a simple implementation of adding a filter function to
@autodocs
via aFilter
keyword (as shortly discussed in slack here and the corresponding thread). See here for an exampleindex.md
where the filtering function is created here. Do you generally agree with that idea?It seems to work if I try it with this example setup https://github.com/timkittel/Documenter-test but I don't really know how I would add the tests here. Could you give me some pointers?
Of course, I would add a sction to the documentation if there is positive feedback here (: